Skip to content

fix: recover stale luks device mapper - #1364

Open
lukasmetzner wants to merge 7 commits into
mainfrom
fix-luks-recover-stale-dev-mapper
Open

lukasmetzner wants to merge 7 commits into
mainfrom
fix-luks-recover-stale-dev-mapper

Conversation

@lukasmetzner

@lukasmetzner lukasmetzner commented May 13, 2026 •

Copy link
Copy Markdown
Contributor

If a Volume is detached without being unpublished (e.g. node crash, forced detach), its dm-crypt mapper scsi-0HC_Volume_<id> stays active while its backing device is gone. cryptsetup status then reports:

/dev/mapper/scsi-0HC_Volume_<id> is active.
  type:    n/a
  cipher:  aes-xts-plain64
  keysize: 512 bits
  device:  (null)
  mode:    read/write

When the Volume is attached again, CryptSetup.Open only checked the exit code of cryptsetup status and reused the stale mapper. mount-utils could not read a filesystem from it, treated it as unformatted and ran mkfs.ext4 -F against the dead device, failing and retrying on every publish.

With this change, Open reuses an active mapper only if its backing device is the device verified in #1457 (same device number). Otherwise, the mapper is closed and opened again on the correct device. If the stale mapper is still mounted, closing it fails and the publish errors out instead of formatting.

@codecov

codecov Bot commented May 13, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.00000% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.72%. Comparing base (653ab38) to head (bde23c3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/volumes/cryptsetup.go 37.50% 30 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1364      +/-   ##
==========================================
- Coverage   79.49%   77.72%   -1.77%     
==========================================
  Files          19       19              
  Lines        1453     1486      +33     
==========================================
  Hits         1155     1155              
- Misses        298      331      +33     
Flag Coverage Δ
e2e-controller 61.30% <16.00%> (-3.53%) ⬇️
e2e-node 61.30% <16.00%> (-3.53%) ⬇️
integration 3.62% <0.00%> (-0.12%) ⬇️
unit 48.18% <24.00%> (-0.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lukasmetzner

Copy link
Copy Markdown
Contributor Author

Debugging information provided by customer indicates there is an issue with luks device mappers not being closed properly. I could not yet reproduce this correctly, but this would address the issue.

@lukasmetzner

lukasmetzner commented May 13, 2026 •

Copy link
Copy Markdown
Contributor Author

As an addition, I properly introduced context management into the cryptsetup. Required a little refactor, but this is mostly just dragging along the context.

@Jank1310

Copy link
Copy Markdown

@lukasmetzner I think we got hit by somthing related to this pr
Happy to share the full hcloud-csi-node logs and cryptsetup status if needed

AI

A dm-crypt mapper left over from an earlier attach cycle stayed active while pointing at a dead dev_t — cryptsetup status reported type: n/a / device: (null). Since Open() short-circuits on IsActive(), which only inspects cryptsetup's exit code and discards its output, the driver reused that zombie mapper; mount-utils then concluded it was unformatted and ran mkfs.ext4 -F against it — 708 times over 23.8 hours, each attempt failing with I/O errors and emitting a fresh Filesystem UUID.

I ran this PR's NewLUKSDeviceStatus/IsZombie() against our verbatim cryptsetup status output and it returns true, so #1364 would have closed the stale mapper and re-opened LUKS against the real device instead of mkfs-ing a dead one.

@lukasmetzner
lukasmetzner force-pushed the fix-luks-recover-stale-dev-mapper branch 2 times, most recently from 8fe7cca to 9e7aa75 Compare October 19, 2026 07:56
@lukasmetzner
lukasmetzner force-pushed the fix-luks-recover-stale-dev-mapper branch from 9e7aa75 to cb75ce8 Compare October 19, 2026 08:11
@lukasmetzner
lukasmetzner marked this pull request as ready for review October 21, 2026 07:34
@lukasmetzner
lukasmetzner requested a review from a team as a code owner October 21, 2026 07:34
func (cs *CryptSetup) IsActive(ctx context.Context, luksDeviceName string) (bool, error) {
output, code, err := command(ctx, cryptsetupExecuable, "status", luksDeviceName)
func (cs *CryptSetup) Status(ctx context.Context, luksDeviceName string) (LUKSDeviceStatus, error) {
output, code, err := cryptsetup(ctx, "status", luksDeviceName)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly replace with a library and don't parse output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants